Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Multi-ASIC] To pass the asic instance ID to orchagent, Advance the swss, swss-common submodules. #4465

Merged
merged 5 commits into from
Apr 29, 2020

Conversation

judyjoseph
Copy link
Contributor

@judyjoseph judyjoseph commented Apr 22, 2020

- Why I did it
The multi ASIC platforms needs support to identify the ASIC instance to be associated with a swss/syncd instance running in a namespace.

- How I did it
This change will go along with sonic-net/sonic-swss#1269. Get the instance ID present in the ConfigDB, DEVICEMETADATA and if it is a non-zero length string, send it along with option -i while starting orchagent.

In addition the swss and swss-common submodules were advanced with the following commits

sonic-swss
3829053 [vnet] Set MTU for the VNET bridge RIF in BITMAP implementation (#1271)
d74c2d7 [vnet] Verify if BITMAP route exists before creating new one to avoid dublication (#1272)
e3f2b29 [routeorch] Handle the empty "nexthop" field for backward compatibility (#1263)
d304673 [Multi-AISC] Pass the asic instance as SAI attribute during switch_create (#1269)
3c8289b Fix obvious syntax errors (#1262)
93d584d Do not set PG to Buffer porfile mapping again if already exist. (#1261)
aa116f4 [dvs] Update tests to use correct command to bring up interfaces (#1260)
53ee2e3 [dvslib] Improve support for assert logging (#1258)
25097d2 [dvs] Stabilize sub-port tests (#1254)
327605a [bufferorch] Fixed SAI_BUFFER_PROFILE_ATTR_BUFFER_SIZE to uint64_t (#1255)
148a220 Update README.md: improve the style of build badges and add LGTM badges (#1251)
5d26ce3 [lgtm]: add lgtm cpp check (#1248)
58627af const initializer_list is not a constant expression (#1250)
1ae9036 [dvs] Re-enable RIF tests (#1249)
4e7e772 [dvs] Refactor VLAN tests to use dvslib (#1241)
eea6815 configure extra inc/lib directory for build (#1247)
f5edec7 Refine getDbId() calling to fix build after swss-common change (#1245)
7505fe0 [lgtm] Add LGTM checks to vs tests (#1244)
319c587 Update README.md (#1240)
8df99da Add more log message, fix test code (#1239)
fc25f82 [vnet]: Fix double route installation for BITMAP VNET interface (#1114)
ae1daf3 [portsorch] Enable port-level buffer drop counters (#1237)
d6236b5 [sub intf] Use m_lag_id to be the parent port object id when parent port is LAG (#1235)
b27d06b [dvs] Add generic polling utility (#1233)
177a6b1 [dvs] Add redis polling interface to dvs fixture (#1228)
841cd69 Don't remove RIF for Vnet interface when ip prefix is deleted (#1225)
bcac081 [tests]: Specify versions of pip packages (#1227)
58e62d7 [dvs] Refactor NAT tests to use redis polling fixtures (#1220)
0de72da [mclag]:add mclagsyncd (#811)
29dc62c [DPB:orchagent:portsorch] Use SAI REDIS return code to track dependency on port (#1219)
a1ff711 [dvs] Mark unstable test cases as xfail (#1223)
10aac0f Add missing netlink include in nbrmgr.cpp (#1221)
9799471 [dvs] Refactor ACL tests to use polling interface to access redis (#1213)
060a44e upon cold reboot, skip remove mgmt vrf table from the kernel (#1214)
586886b Multi-Db changes for NAT feature. (#1202)
884507b [ACL/DPB] Add support for in-place updates to ACL table bindings (#1148)
0acf65d using PRI instead of %l to support 32 bit arch (#1162)
fb8b6a0 [vnet]: Fix continues warning messages after config reload (#1217)
87c86ba Default action for Egress ACL Table not poulated. (#1208)
574f199 Add/Del lag_name_map item according to lag adding and removing (#1124)
92c7c33 Enable m_isCombinedMirrorV6Table for BFN platform (#1212)
3f2b112 [dvs] Re-enable NAT test cases (#1205)
2c243d7 Removed dead code (#1198)
9d065b7 [dvs] Add NAT tests back to test suite (#1200)
10d2e73 [dvs] Update instructions for running dvs tests locally (#1196)
853d822 [intfsorch]:add support to change rif mac address (#814)
88f7a2a [team sync/mgr] Add debug message before cleaning up LAGs (#1192)

sonic-swss-common
889c0a Fix backward compatible for a DBConnector without a dbName (#340)
528aede Update README.txt: add LGTM badges (#338)
fef68a9 Fix test config (#337)
bfcb936 [DBConnector] Add methods to set/get Redis client name (#335)
b2c6edd [MultiDB]: Use table name separator from database_config.json, instead of hardcoded in a map (#336)
f85b5ae Clean fdb flush test code (#333)
e6d5c1a Clean netlink includes (#332)
8b54767 Add fdb flush lua script v2 with switch id support (#329)
7cc1c5a Add stdexcept lib to solve error when build pkdg (#328)

- How to verify it
Verified on a multi-asic platform and a single asic platform.

- A picture of a cute animal (not mandatory but encouraged)

…hich will be pulled and

will be used when starting the orchagent process with the new option [-i INST_ID]
This is currently added only for Broadcom ASIC based platforms
@judyjoseph
Copy link
Contributor Author

judyjoseph commented Apr 22, 2020

retest vsimage please

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it generic to all platforms.

@lguohan
Copy link
Collaborator

lguohan commented Apr 23, 2020

since the sonic-swss is merged, can you update the submodule in this pr.

@lguohan
Copy link
Collaborator

lguohan commented Apr 25, 2020

thanks for update swss and swss-common. in pr with submodule update, can you also list the commits that went to those submodules in your commit message?

@judyjoseph judyjoseph changed the title [Multi-ASIC] To pass the asic instance ID when starting orchagent process [Multi-ASIC] To pass the asic instance ID to orchagent, Advance the swss, swss-common submodules. Apr 25, 2020
@judyjoseph judyjoseph requested a review from lguohan April 27, 2020 05:15
@judyjoseph
Copy link
Contributor Author

make it generic to all platforms.

It was made generic to all platforms. Please take a look.

asic_id=`sonic-cfggen -d -v DEVICE_METADATA.localhost.asic_id`
if [ -n "$asic_id" ]
then
ORCHAGENT_ARGS+="-i $asic_id "
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$asic_id [](start = 24, length = 8)

There is security risk that attacker tamper the asic_id in Redis and run any command.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i put this as a todo list.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure Guohan, will take it up as a follow up action. But wanted to add one more point to this.

Currently we pass the instance ID as integers 0,1,2. It will be changed in future to pass the PCI device ID's when we update the new SAI will accepts PCI device IDs. The config DB will be populated with PCI device ID's in this field "DEVICE_METADATA.localhost.asic_id" and will be passed on when starting orchagent process.

This should be safe ? if we pass a wrong PCI device id ( may be by an attacker) ... the SAI create switch will fail and syncd won't come up...

Copy link
Collaborator

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants